Senior engineer review

Good direction, but I would not merge this as written. This changes the contributor contract repo-wide, and the current implementation is stricter than the ADR tradeoff section says is appropriate.

Suggested changes

1. .github/pull_request_template.md:27 / CONTRIBUTING.md:17

   The template and guide require full OpenClaw/Hermes prior-art research for every PR. That is too broad for docs fixes, dependency bumps, release PRs, trivial CI fixes, and small bug fixes. The ADR itself calls out the strict-everywhere option as high-friction and says a tiered policy is the recommended follow-up.

   Recommendation: make prior-art research mandatory for architectural or behavioral changes, and allow a short Not applicable answer for docs/chore/release/minor fixes. This preserves the quality bar without turning small contributions into ceremony.

2. .github/pull_request_template.md:57 / CONTRIBUTING.md:38

   The validation section is Rust-only: cargo check and cargo test. The repo now has Helm charts, GitHub Actions, gateway code, docs-only PRs, and release automation. A required cargo checklist is wrong or irrelevant for several valid PR types.

   Recommendation: change this to Relevant validation with examples: Rust checks for Rust changes, helm lint / helm unittest / helm template for chart changes, workflow checks for CI changes, and explicit not applicable for docs-only changes.

3. CONTRIBUTING.md:7 / docs/adr/pr-contribution-guidelines.md:83

   The ADR says CONTRIBUTING.md links to the ADR, but the contributed CONTRIBUTING.md only links to the PR template. Add a link to docs/adr/pr-contribution-guidelines.md or adjust the ADR implementation table.

Nit

• .github/pull_request_template.md:29 and .github/pull_request_template.md:48 have trailing whitespace. git diff --check origin/main...HEAD fails on both lines.

Verdict

Advance the concept, but revise before merge. The right shape is a tiered PR template and contributor guide that matches the current repo surfaces and existing steering docs, not a one-size-fits-all template.

PR Review: #302

Summary

• Problem: The project lacks a standardized PR template and contribution guidelines, leading to inconsistent PR quality and research burden on reviewers.
• Approach: Adds an ADR, a GitHub PR template, and a CONTRIBUTING.md requiring prior art research (OpenClaw + Hermes Agent) for every PR.
• Risk level: Low (docs-only, no runtime changes)

Core Assessment

1. Problem clearly stated: ✅ — well-motivated, with a concrete good example (PR #225)
2. Approach appropriate: ⚠️ — good direction, but the current implementation applies the heaviest process uniformly to all PR types (see findings)
3. Alternatives considered: ✅ — ADR Section 4 lists three options and identifies tiered policy as the recommended follow-up
4. Best approach for now: ❌ — the ADR itself acknowledges tiered policy is better, but the template and CONTRIBUTING.md implement the strict-everywhere option instead

Findings

I agree with @shaun-agent's review on the core issues. Here are my detailed findings:

.github/pull_request_template.md

Lines 27–42 — Prior Art section is mandatory for all PRs

The template requires OpenClaw and Hermes Agent research for every PR. This is excessive for:

• Docs-only changes (typo fixes, README updates)
• Dependency bumps / Cargo.lock updates
• CI/CD workflow changes
• Release PRs
• Trivial bug fixes (one-liner fixes with obvious cause)

The ADR itself calls this out in Section 4 Option 2 as "excessive overhead for trivial/docs/small fixes" and recommends Option 3 (tiered policy) as a follow-up. The template should implement the tiered approach from the start.

Lines 57–64 — Validation section is Rust-only

The checklist hardcodes cargo check and cargo test. The repo has multiple surfaces:

• Rust code → cargo check, cargo test, cargo clippy
• Helm charts → helm lint, helm template
• GitHub Actions → workflow syntax validation
• Docs → no cargo validation needed

A Rust-only checklist is misleading for non-Rust PRs and will be ignored or confusingly checked off as N/A.

Lines 29, 48 — Trailing whitespace

git diff --check would flag these. Minor, but worth cleaning up.

CONTRIBUTING.md

Line 10 — Discord Discussion URL auto-close policy

"PRs without one will be labeled closing-soon and auto-closed after 3 days."

This is a strong policy that could discourage first-time contributors. External contributors may not be in the Discord server. Consider making this "strongly recommended" rather than auto-close-worthy, or provide an alternative (e.g., "If no Discord discussion exists, explain why in the PR description").

Lines 17–26 — Same prior art issue as the template

Repeats the mandatory-for-all-PRs requirement. Should match whatever tiered approach the template adopts.

Lines 38–40 — Rust-only test plan

Same issue as the template — cargo check and cargo test are not relevant for all PR types.

Missing link to ADR

The ADR implementation table (Section 3, Phase 2) says CONTRIBUTING.md should link to the ADR. Currently it only links to the PR template. Add a link to docs/adr/pr-contribution-guidelines.md.

docs/adr/pr-contribution-guidelines.md

Overall structure: ✅ Follows the existing ADR format (matches custom-gateway.md, line-adapter.md). Well-organized with clear sections.

Line 83 — Implementation table

Phase 2 says CONTRIBUTING.md links to this ADR, but the actual CONTRIBUTING.md does not. Implementation should match the spec.

Section 4 — Alternatives Considered

Good analysis. Option 3 (tiered policy) is correctly identified as the recommended follow-up. The issue is that the PR implements Option 2 while recommending Option 3. This inconsistency should be resolved — either implement the tiered approach now, or explicitly state in the ADR that this PR ships Option 2 as an interim step with a tracked issue for Option 3.

Review Summary

🔴 Blockers

(none — this is a docs PR with no runtime impact, but the following items should be addressed before merge)

🔧 Suggested Changes

1. Implement tiered PR requirements — Make prior art research mandatory only for architectural/behavioral changes. For docs, chore, CI, release, and minor fixes, allow a short "Not applicable — [reason]" in the prior art section. The ADR already recommends this.
2. Make validation section multi-surface — Replace the Rust-only checklist with a "Relevant validation" section that lists examples per PR type (Rust, Helm, CI, docs).
3. Soften Discord Discussion URL policy — Change from auto-close to "strongly recommended" or provide an alternative path for external contributors not in Discord.
4. Add ADR link to CONTRIBUTING.md — As specified in the ADR implementation table.
5. Fix trailing whitespace — Lines 29 and 48 of the PR template.

💬 Questions

1. Is there an existing issue tracking the tiered policy follow-up? If not, should one be opened alongside this PR?
2. The ADR references OpenClaw and Hermes Agent as mandatory research targets — are these the right two projects long-term, or should the list be configurable/expandable?

ℹ️ Info

• This is the first PR template and CONTRIBUTING.md for the repo — currently neither exists on main.
• The ADR format is consistent with existing ADRs (custom-gateway.md, line-adapter.md).
• @shaun-agent's review raised the same core concerns (tiered policy, Rust-only validation, missing ADR link). My review aligns with and reinforces those findings.

⚪ Nits

• Trailing whitespace on PR template lines 29 and 48.

Verdict

COMMENT — Good direction and well-structured ADR. The core concept (standardized PR guidelines with prior art research) is valuable. However, the implementation should match the ADR's own recommendation for a tiered approach rather than applying the strictest policy uniformly. Address the suggested changes above, and this will be ready for another look.